Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend VMware provisioning tests for OS version check #14094

Closed
wants to merge 1 commit into from
Closed

Extend VMware provisioning tests for OS version check #14094

wants to merge 1 commit into from
+52 −13

Conversation

Gauravtalreja1
Copy link
Collaborator

Problem Statement

Bootdisk provisioning passes for VMware 8, but installed status isn't updated, we've BZ:2255264 reported for it,

Solution

Handling BZ:2255264 in automation while this is open, and extending the test to check OS version when provisioning is completed for all methods including bootdisk

@Gauravtalreja1 Gauravtalreja1 added TestFailure Issues and PRs related to a test failing in automation CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Feb 15, 2024
@Gauravtalreja1 Gauravtalreja1 self-assigned this Feb 15, 2024
@Gauravtalreja1 Gauravtalreja1 requested a review from a team as a code owner February 15, 2024 13:34
lhellebr
lhellebr reviewed Feb 15, 2024
View reviewed changes
Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't need to sat.execute and from there ssh to the host. You should be able to execute on the host directly, shouldn't you?
Otherwise LGTM.

tests/foreman/cli/test_computeresource_vmware.py Show resolved Hide resolved
tests/foreman/cli/test_computeresource_vmware.py Show resolved Hide resolved
@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Feb 15, 2024
@Gauravtalreja1 Gauravtalreja1 force-pushed the handle-vcenter8-bootdisk-bz branch from 9593bb3 to bc1ca38 Compare February 15, 2024 21:21
@shubhamsg199
Copy link
Contributor

trigger: test-robottelo
pytest: tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
provisioning: true

@Gauravtalreja1
Copy link
Collaborator Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
provisioning: true

@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 22, 2024
@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 22, 2024
@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 22, 2024
@SatelliteQE SatelliteQE deleted a comment from Satellite-QE Feb 22, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 5825
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
Test Result : =========================== short test summary info ============================

@Gauravtalreja1 Gauravtalreja1 mentioned this pull request Feb 28, 2024
Closed
@Gauravtalreja1 Gauravtalreja1 added No-CherryPick PR doesnt need CherryPick to previous branches and removed CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Feb 28, 2024
@Gauravtalreja1 Gauravtalreja1 force-pushed the handle-vcenter8-bootdisk-bz branch from 62ef2d8 to 5e1ef99 Compare April 7, 2024 08:40
@Gauravtalreja1
Copy link
Collaborator Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
provisioning: true

@Satellite-QE Satellite-QE removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 7, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6331
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end --external-logging --include-onprem-provisioning
Test Result : === 10 failed, 14 passed, 2 deselected, 454 warnings in 20493.66s (5:41:33) ====

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 7, 2024
@Gauravtalreja1 Gauravtalreja1 force-pushed the handle-vcenter8-bootdisk-bz branch from 5e1ef99 to 89a9d09 Compare April 10, 2024 12:29
@Gauravtalreja1 Gauravtalreja1 requested a review from a team as a code owner April 10, 2024 12:29
@Gauravtalreja1
Copy link
Collaborator Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
provisioning: true

@Satellite-QE Satellite-QE removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6417
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end --external-logging --include-onprem-provisioning
Test Result : ========== 2 failed, 2 deselected, 146 warnings in 3617.79s (1:00:17) ==========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
ogajduse
ogajduse reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@ogajduse ogajduse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

co-reviewed with @rmynar

What is the status of this PR? Do you need any help with this one?
From what we see, [E 240410 13:32:18 exceptions:17] AuthenticationError: Key-based authentication failed. pops up when the test tries to connect to the host using broker.
Did you try to investigate the server logs? Enabling debug logging on SSHD where you are trying to connect is usually helpful in debugging these sorts of issues.

Also since the workaround is not present in the patch, I would recommend changing the PR title.

@Gauravtalreja1 Gauravtalreja1 changed the title Add workaround for BZ:2255264 and extend for OS version check Extend VMware provisioning tests for OS version check Apr 22, 2024
@Gauravtalreja1 Gauravtalreja1 force-pushed the handle-vcenter8-bootdisk-bz branch from 89a9d09 to c180aa1 Compare May 8, 2024 21:50
@Gauravtalreja1
Copy link
Collaborator Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end
provisioning: true

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6849
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end --external-logging --include-onprem-provisioning
Test Result : ========== 2 failed, 2 deselected, 145 warnings in 4594.32s (1:16:34) ==========

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6850
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_computeresource_vmware.py -k test_positive_provision_end_to_end --external-logging --include-onprem-provisioning
Test Result : ========== 2 failed, 2 deselected, 145 warnings in 4915.86s (1:21:55) ==========

rplevka
rplevka requested changes May 17, 2024
View reviewed changes
robottelo/hosts.py
Comment on lines +461 to +469
else:
wait_for(
self.connect,
fail_condition=lambda res: res is not None,
handle_exception=True,
raise_original=True,
timeout=timeout,
delay=1,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this actually necessary?
isn't
lambda: self.connect(key_filename=self.key_filename), universal for both cases?
you could still pass key_filename=None if it is not specified and it should work, shouldn't it?
https://github.com/SatelliteQE/broker/blob/master/broker/hosts.py#L117

tests/foreman/cli/test_computeresource_vmware.py
@pytest.mark.parametrize('vmware', ['vmware8'], indirect=True)
@pytest.mark.parametrize('pxe_loader', ['uefi'], indirect=True)
@pytest.mark.parametrize('provision_method', ['build'])
@pytest.mark.rhel_ver_match('[8, 9]')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i noticed you have a hardcoded rhel8 here:
https://github.com/SatelliteQE/robottelo/pull/14094/files#diff-f5876fe03942ff6e50c18a8f2c95255cdca6f84b9956721a587abf7322fb73abR130
mind changing that, so it reflects the current rhel_ver parameter?

tests/foreman/cli/test_computeresource_vmware.py
provisioning_host = ContentHost(
host_info['network-interfaces'][0]['ipv4-address'], auth=module_ssh_key_file
)
expected_rhel_version = host_info['operating-system']['operating-system'][-3:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are setting yourself for a failure with this kind of version parsing

In [1]: '8.10'[-3:]
Out[1]: '.10'

also, that field might contain os name + version + some suffix, like beta or possibly eus.
I'd suggest to use nailgun to get the host info, which should resolve the os entity where you can directly access major and minor versions.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2024

This pull request has not been updated in the past 45 days.

@github-actions github-actions bot added the Stale Stale issue or Pull Request label Jul 2, 2024
@Gauravtalreja1 Gauravtalreja1 removed the Stale Stale issue or Pull Request label Jul 9, 2024
@jameerpathan111
Copy link
Contributor

@Gauravtalreja1 I'm converting pr to draft for now, please mark it ready for review once you're done with changes/addressing the comments.

@jameerpathan111 jameerpathan111 marked this pull request as draft August 20, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shubhamsg199 shubhamsg199 shubhamsg199 left review comments

@ogajduse ogajduse ogajduse left review comments

@rplevka rplevka rplevka requested changes

@lhellebr lhellebr lhellebr approved these changes

Assignees

@Gauravtalreja1 Gauravtalreja1

Labels
No-CherryPick PR doesnt need CherryPick to previous branches PRT-Failed Indicates that latest PRT run is failed for the PR TestFailure Issues and PRs related to a test failing in automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Gauravtalreja1 @shubhamsg199 @Satellite-QE @jameerpathan111 @lhellebr @rplevka @ogajduse